Skip to content

implement aks gpu instance profile#3895

Merged
zhoxing-ms merged 5 commits intoAzure:mainfrom
alexeldeib:ace/mig
Oct 2, 2021
Merged

implement aks gpu instance profile#3895
zhoxing-ms merged 5 commits intoAzure:mainfrom
alexeldeib:ace/mig

Conversation

@alexeldeib
Copy link
Contributor

@alexeldeib alexeldeib commented Sep 16, 2021

This checklist is used to make sure that common guidelines for a pull request are followed.

General Guidelines

  • Have you run azdev style <YOUR_EXT> locally? (pip install azdev required)
    • this fails but not on anything related to my diff.
  • Have you run python scripts/ci/test_index.py -q locally?

For new extensions:

About Extension Publish

There is a pipeline to automatically build, upload and publish extension wheels.
Once your PR is merged into master branch, a new PR will be created to update src/index.json automatically.
The precondition is to put your code inside this repo and upgrade the version in the PR but do not modify src/index.json.

@alexeldeib alexeldeib force-pushed the ace/mig branch 4 times, most recently from f3435c6 to ccbe8cb Compare September 16, 2021 01:09
@yonzhan
Copy link
Collaborator

yonzhan commented Sep 16, 2021

aks

@yonzhan yonzhan added this to the Sep 2021 (2021-10-05) milestone Sep 16, 2021
@FumingZhang
Copy link
Member

FumingZhang commented Sep 16, 2021

It seems that the live test pipeline failed on a removed test case test_aks_nodepool_add_with_workload_runtime, but test_aks_nodepool_add_with_gpu_instance_profile is not tested? Could you please queue another test? If the newly added option needs feature registration, you may exclude this test case in ext_matrix_default.json.

@alexeldeib
Copy link
Contributor Author

alexeldeib commented Sep 16, 2021

ah sorry, I mixed up the tests with this and #3896

@alexeldeib
Copy link
Contributor Author

alexeldeib commented Sep 16, 2021

I'll exclude the live test due to feature registration, unfortunately same issue applies to #3896

FumingZhang
FumingZhang reviewed Sep 16, 2021
View reviewed changes
src/aks-preview/azext_aks_preview/_params.py Outdated
Copy link
Member

@FumingZhang FumingZhang Sep 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May consider using a variable to wrap these options?

@alexeldeib
Copy link
Contributor Author

alexeldeib commented Sep 23, 2021

@FumingZhang anything we're waiting on here for merge?

@zhoxing-ms
Copy link
Contributor

zhoxing-ms commented Sep 24, 2021 •
edited
Loading

@alexeldeib Please write those changes into the HISTORY.rst

By the way, do you need to release a new version for these changes? If so, please also update the setup.py

@zhoxing-ms
Copy link
Contributor

zhoxing-ms commented Sep 29, 2021

@alexeldeib Could you please address those conflicts?

@alexeldeib
Copy link
Contributor Author

alexeldeib commented Sep 30, 2021

rebased 👍

@zhoxing-ms zhoxing-ms merged commit 995397c into Azure:main Oct 2, 2021
@alexeldeib alexeldeib mentioned this pull request Oct 13, 2021
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@FumingZhang FumingZhang FumingZhang approved these changes

@andyliuliming andyliuliming Awaiting requested review from andyliuliming andyliuliming is a code owner

@zqingqing1 zqingqing1 Awaiting requested review from zqingqing1

@zhoxing-ms zhoxing-ms Awaiting requested review from zhoxing-ms

@jsntcy jsntcy Awaiting requested review from jsntcy

Assignees

@zhoxing-ms zhoxing-ms

Labels

None yet

Projects

None yet

Milestone

Sep 2021 (2021-10-05)

Development

Successfully merging this pull request may close these issues.

4 participants

@alexeldeib @yonzhan @FumingZhang @zhoxing-ms